test: add overflow tests for MutableBuffer#9958
Conversation
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks good to me -- thank you @SoimanVasile 🙏
|
I merged up to hopefully get a clean CI run |
|
Thanks again @SoimanVasile |
|
@alamb Thanks so much for the review and the merge! I'm relatively new to open-source contributing, but I really enjoyed diving into the MutableBuffer logic here. |
|
Thanks @SoimanVasile -- I don't know of any small tickets yet. Maybe you could help by looking at some of the PRs that are outstanding / looking for review and help. PRs that are small and focused are the ones that we are likely to be able to review quickly |
### Description While analyzing the memory allocation logic in `MutableBuffer`, I identified that the `with_capacity` and `reserve` methods correctly use `.expect()` guards to prevent integer overflows. However, I couldnt find test cases for this `.expect()` guard in the current test suite. This PR adds 2 `#[should_panic]` tests in `mutable.rs` to verify that the API correctly panics **Changes:** * Added `test_mutable_new_capacity_overflow` to cover `MutableBuffer::new` * Added `test_mutable_reserve_overflow` to cover `MutableBuffer::reserve` ### Rationale Adding these tests ensures that the safety guards in `arrow-buffer` remain intact and provides explicit coverage for edge cases involving near-`usize::MAX` allocations. ### Tests - `test_mutable_new_capacity_overflow` - `test_mutable_reserve_overflow` Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
|
@alamb Sounds good! I will definitely look into the outstanding PRs next week when I have a bit more time to focus on them. Thanks for the guidance! |
2 participants
Description
While analyzing the memory allocation logic in
MutableBuffer, I identified that thewith_capacityandreservemethods correctly use.expect()guards to prevent integer overflows.However, I couldnt find test cases for this
.expect()guard in the current test suite. This PR adds 2#[should_panic]tests inmutable.rsto verify that the API correctly panicsChanges:
test_mutable_new_capacity_overflowto coverMutableBuffer::newtest_mutable_reserve_overflowto coverMutableBuffer::reserveRationale
Adding these tests ensures that the safety guards in
arrow-bufferremain intact and provides explicit coverage for edge cases involving near-usize::MAXallocations.Tests
test_mutable_new_capacity_overflowtest_mutable_reserve_overflow